-
Notifications
You must be signed in to change notification settings - Fork 1.1k
No warn implicit param of overriding method #22901
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
No warn implicit param of overriding method #22901
Conversation
ce0375f
to
819586f
Compare
Redrafting to also inspect self-types (for signatures constrained by super- or self-type). Noticed this because of the test from Scala 2, which had an explicit override as in U2. That stopped warning in this commit. //> using options -Wunused:params
trait T {
def f(x: Int, y: Int): Int
def i: Int
}
trait U {
this: T =>
def f(x: Int, y: Int): Int = x*i
def g(x: Int, y: Int): Int = x*i
}
trait U2 {
this: T =>
override def f(x: Int, y: Int): Int = x*i
}
trait V extends T {
def f(x: Int, y: Int): Int = x*i
def g(x: Int, y: Int): Int = x*i
} 2.13 warns about g only. 3.7 also warns about U.f (because it's not looking at self-type). 3.6.4 emits no warning for some reason I will not investigate. (Edit: guessing maybe because "an overriding method might use the param"? but that would make the check mostly useless.) |
About the "heuristics": it's difficult to write a test with confidence, because you don't know when a heuristic kicks in. (See comments about avoiding "trivial RHS".) That would be easier with strict mode. Users could enable strict only for C.I. |
The suggestion to try nightlies would've saved me about 10 minutes because I would've seen @som-snytt already fixed #22901... [skip ci] --------- Co-authored-by: Som Snytt <[email protected]> Co-authored-by: Piotr Chabelski <[email protected]>
Fixes #22895
Fixes #22896
Follow-up to #22729
Follow-up to #22757
Since Scala 2, the check for unused parameters does not warn for an overriding method, since the signature is dictated by the overridden method. That behavior was intentionally omitted in 3.7 because it was introduced in Scala 2 before
@nowarn
and@unused
were available. (The warning would be too noisy without mitigation.) A compiler option for these heuristics was tried in a previous commit, but reverted. (Compiler options are difficult to evolve once added.) So these commits restore this heuristic without further ado. An option for "strict mode" without allowances is future work.The other issue was that detecting an override in a Java parent did not work. The commit always checks
is(Override)
before searching for an overridden member, since it is not necessary to verify the modifier (which happens in RefChecks). Incorrect overriding would result in false negatives, which is acceptable since it will error later.Further asymmetry between checking implicit params and explicit class param accessors may or may not be dubious.